Skip to content

Add tests for ALTER COLUMN to assert that column default expressions are resolved #1294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 18, 2025
Merged

Add tests for ALTER COLUMN to assert that column default expressions are resolved #1294

merged 1 commit into from
Mar 18, 2025

Conversation

@fulghum
Copy link
Contributor

@fulghum fulghum commented Mar 17, 2025

Altering a column with an existing column default expression was triggering a panic in Doltgres, because Doltgres' TypeSanitizer was finding an UnresolvedColumnDefault instance in the plan.ModifyColumn instances's column field.

This is not an issue from Dolt or GMS, because they don't support MySQL's syntax to alter a single component of an existing column, without requiring its full column definition to be respecified. (And also because they don't use Doltgres' TypeSanitizer.)

Depends on dolthub/dolt#8994
Depends on dolthub/go-mysql-server#2895

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2025 •
edited
Loading

Main PR
covering_index_scan_postgres 391.77/s 388.77/s -0.8%
index_join_postgres 156.03/s 156.21/s +0.1%
index_join_scan_postgres 187.85/s 187.71/s -0.1%
index_scan_postgres 12.62/s 12.65/s +0.2%
oltp_point_select 2808.27/s 2793.37/s -0.6%
oltp_read_only 1888.83/s 1866.86/s -1.2%
select_random_points 111.50/s 111.86/s +0.3%
select_random_ranges 135.92/s 134.32/s -1.2%
table_scan_postgres 10.54/s 10.40/s -1.4%
types_table_scan_postgres 5.40/s 5.45/s +0.9%

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2025 •
edited
Loading

Main PR
Total 42090 42090
Successful 15732 15732
Failures 26358 26358
Partial Successes1 5190 5190
Main PR
Successful 37.3770% 37.3770%
Failures 62.6230% 62.6230%

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

zachmu
zachmu approved these changes Mar 17, 2025
View reviewed changes
Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fulghum fulghum force-pushed the fulghum/alter_column branch from 792407c to c2ac333 Compare March 18, 2025 02:16
@fulghum fulghum merged commit b1aac16 into main Mar 18, 2025
14 checks passed
@fulghum fulghum deleted the fulghum/alter_column branch March 18, 2025 02:41
@BrewTestBot BrewTestBot mentioned this pull request Apr 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zachmu zachmu zachmu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fulghum @zachmu